Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Inbound field to admin_peers rpc call response #7443

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

obasekiosa
Copy link
Contributor

@obasekiosa obasekiosa commented Sep 16, 2024

Closes #7317

Changes

  • Adds Inbound boolean field which specifies if the peer is inbound or not to the PeerInfo Field

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Previous tests where smoke tests and redundant, followed the pattern.

Documentation

Requires documentation update

  • Yes
  • No

Link to docs update: NethermindEth/docs#223

Requires explanation in Release Notes

  • Yes
  • No

@@ -43,13 +45,13 @@ public PeerInfo(Peer peer, bool includeDetails)
IsBootnode = peer.Node.IsBootnode;
IsStatic = peer.Node.IsStatic;
Enode = peer.Node.ToString(Node.Format.ENode);
Inbound = peer.InSession is not null && peer.InSession.Direction == ConnectionDirection.In; // assumes one of InSession/Outsession is always set.?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all InSession should have direction.In

EthDetails = peer.Node.EthDetails;
LastSignal = (peer.InSession ?? peer.OutSession)?.LastPingUtc.ToString(CultureInfo.InvariantCulture);
}
if (!includeDetails) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal: I dislike jumping out from methods in the middle (it is okayish at the begining).

@@ -15,7 +15,7 @@ public class PeerInfo
public string Host { get; set; }
public int Port { get; set; }
public string Address { get; set; }
public bool IsBootnode { get; set; }
public bool IsBootnode { get; set; } // change to BootNode (capitalize "N")?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change, but what is the standard?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add inbound field for admin_peers json-rpc call.
2 participants